Rename map_entities and map_specific_entities #7570

Testare · 2023-02-08T18:46:50Z

Objective

After fixing dynamic scene to only map specific entities, we want map_entities to default to the less error prone behavior and have the previous behavior renamed to "map_all_entities." As this is a breaking change, it could not be pushed out with the bug fix.

Solution

Simple rename and refactor.

Changelog

Changed

map_entities now accepts a list of entities to apply to, with map_all_entities retaining previous behavior of applying to all entities in the map.

Migration Guide

In bevy_ecs, ReflectMapEntities::map_entites now requires an additional entities parameter to specify which entities it applies to. To keep the old behavior, use the new ReflectMapEntities::map_all_entities, but consider if passing the entities in specifically might be better for your use case to avoid bugs.

Testare · 2023-02-08T19:29:00Z

markdownlint failed because of github throttling

Error: fatal: unable to access 'https://github.com/bevyengine/bevy/': The requested URL returned error: 429

(429 means too many request)

Is there a way to rerun the check?

JMS55 · 2023-02-08T22:03:01Z

bors try-

bors · 2023-02-08T22:15:52Z

try

Build succeeded:

Illiux · 2023-02-11T22:44:14Z

I don't think this approach is robust. It's possible that an entity has both a DynamicScene-added Parent component and some other component that implements MapEntities. In that case, I believe, you'd have to somehow map every component on that entity except the Parent component. Your current changes would map every component on the entity including Parent and cause the same bugged behavior. Maybe the better approach is to remove all the Parent components before applying the scene then re-add them according to normal logic.

Testare · 2023-02-11T23:20:24Z

Parent is used as an example, since it is the most pertinent (Parent is added to entities automatically through scene load and causes panic when set improperly, hence how this bug came to my attention), but I did write the code to solve this issue for any MapEntities component.

Basically, the code works by making sure that when a dynamic scene is loaded, it keeps track of all the MapEntities components (by TypeID and Entity), and makes sure that only the MapEntities components that are loaded from the scene are invoked with the entities map.

So let's make a theoretical component struct StepFather(Entity) that implements MapEntities. If this component is added to a scene file, it will be affected by scene file changes. But if another system were to add this component to scene entities, unless somebody edits the scene file to add the component to the scene file, MapEntities won't be invoked (And in this case, we would WANT MapEntities invoked).

Importantly, this also works even if some components in the scene have StepFather components loaded from files, and others have StepFather components added through systems.

As for the approach of removing parent components and re-adding them, I /did/ try that and it seemed to work fine for Parents-only, though the UI seemed to do weird things (I think children order might not have been preserved). But the big issue was as you've said, that approach only fixes the problem for Parent components, other MapEntities components added systemically would suffer the same issues.

Illiux · 2023-02-11T23:23:26Z

So let's make a theoretical component struct StepFather(Entity) that implements MapEntities. If this component is added to a scene file, it will be affected by scene file changes. But if another system were to add this component to scene entities, unless somebody edits the scene file to add the component to the scene file, MapEntities won't be invoked (And in this case, we would WANT MapEntities invoked).

The issue would be if the scene has a StepFather component and then some other system adds a Parent component to the scene entities. MapEntities gets invoked on Parent when Parent isn't in the scene.

Illiux · 2023-02-11T23:26:38Z

Oh wait, nevermind I see. I was misreading the code and didn't properly notice that the entities list is per-component.

Testare · 2023-02-12T04:59:49Z

Oh hahaha yeah i can see how that would be easy to miss XP Do you think it's fine then?

Testare · 2023-02-16T00:50:19Z

Okay, I'm finding it hard to communicate technically what this bug is and what I did to solve it... So I wrote it out in story/table form! Hopefully it's more entertaining AND clear this way.

Note: The component P in this story is loosely based on Parent, so if it helps you visualize it you can think of P that way. But I wrote it as P because the bug affects all MapEntities components; P could just as easily stand for GankedLastBy, CrushOfTheWeek, or SecretSanta.

The Scene-Reload Bug In 3 Acts: A Visual Journey

Click to read the story

Act I: Setting Up

Let's say we have a component type P. P components contain references to other entities, and so P implements the MapEntities trait in order for these references to be maintained when loaded from scenes or across networks.

Now, you have a world with one entity, and the only component it has is a name. It does not have a P component. I'll illustrate this with a table

World

Entity	P	Name
0	X	Alice

Now you have a scene file that describes two named entities, and one of them has a P component referencing the other

Scene

Entity	P	Name
0	X	Bob
1	0	Charlie

This scene is then loaded into the world with a scene loader. After it has loaded the components, but not applied ReflectMapEntities logic, it looks like this:

World

Entity	P	Name
0	X	Alice
1	X	Bob
2	0	Charlie

Now you'll notice that the P Component is not actually referencing Bob, like the scene defined, since obviously Entity 0 was already taken in the world so the entities loaded from the scene must be given new ones. But while those components were loaded, an EntityMap was created that shows the relationship from the Entity defined in the Scene to the actual Entity in the world.

Entity Map

Scene Entity	World Entity
0	1
1	2

Since P implements MapEntities, it has logic to correct the entity reference using this map. This logic is invoked while the scene is being loaded, applying to all P components where their entity is defined in the EntityMap (1 and 2), so that it looks like this:

World

Entity	P	Name
0	X	Alice
1	X	Bob
2	1	Charlie

Now things look exactly as they should! Bob is Charlie's P, whatever that means.

Act II: The Bug Revealed

Now, things happen in the world that might not be exactly as described in the Scene. Alice, who the scene knew nothing about, becomes Bob's P.

World

Entity	P	Name
0	X	Alice
1	0	Bob
2	1	Charlie

These things happen. Then, we decide that Bob should really go by his full name Robert. We update the scene...

Scene

Entity	P	Name
0	X	Robert
1	0	Charlie

...Then the scene is reloaded! Before we apply MapEntities logic, the World looks like this:

World

Entity	P	Name
0	X	Alice
1	0	Robert
2	0	Charlie

Now a couple things to note. Fortunately for us, scene reloading does not remove or even affect components that aren't defined explicitly in the scene. That means that Alice is safe as Robert's P, even though his name has been changed. This is good, since a large part of the draw of hot-reloading is that state is otherwise unaffected by the reload.

However, Charlie isn't so lucky. Since his P is defined in the Scene, it gets updated to the value defined in the Scene, back to 0. But don't worry Charlie, we still have that EntityMap from before! So let's update P for our scene entities...

World

Entity	P	Name
0	X	Alice
1	1	Robert
2	1	Charlie

There we go Charlie! You're now back to having Robert as your P, all is right with the World-

Wait, no! Alice and Robert are mortified! Alice is no longer Robert's P. Instead Robert's P is... Robert himself?! What will he neighbors think?! There will be shouts of Malformed hierarchy!

It turns out that when we were going around correcting P for all the entities in the scene, we saw that Robert had a P component. Thinking it must still contain a scene entity, we applied the map to point at a real world Entity. But P isn't defined in the scene, so it already had a proper world Entity defined, and now we've gone and set the pointer to some other random entity defined in the scene, in this case the selfsame entity.

Act III: Happily ever After

So here's the solution: Let's turn back time. This time, when we're going to correct the P component with ReflectMapEntities, we'll give it a bit of extra information. We'll make sure that it'll only correct P when the P was actually loaded from the Scene. In this case that's a very short list, a list of only 2.

World

Entity	P	Name
0	X	Alice
1	0	Robert
2	1	Charlie

There we go. Life as we intended it.

Interview with the Author: Handling Multiple Component Types

Interviewer: People often ask, what if there were two components that implemented MapEntities? What if there was another Component, let's say C, and Bob/Robert DID have that component defined in the scene?

Author: Well let's say we defined the scene like this:

Scene

Entity	P	C	Name
0	X	1	Bob
1	0	X	Charlie

Then we skipped ahead in the story to the part where the bug first appeared, upon loading the scene but before updating the entities, it would look something like this:

World

Entity	P	C	Name
0	X	1	Alice
1	0	1	Robert
2	0	X	Charlie

I took the liberty of assuming Robert would be Alice's C, it seems to make sense for their characters. Now people could worry that if we only had the EntityMap applied to entity 2, Charlie, then we would have a similar situation to before, where Robert is his own C.

World

Entity	P	C	Name
0	X	1	Alice
1	0	1	Robert
2	1	X	Charlie

But really the solution to this is quite simple. The MapEntities logic is already called on a per-component-type basis. It goes down each column individually. So we just have to maintain a different entity list for each component, like a HashMap<ComponentId, Vec<Entity>>. So for the scene I wrote out earlier, the list for component P would only contain 2, Charlie, and the list for Component C would only contain 1, Robert. So we would only update those two specific components, those "cells" in the table, so to speak.

World

Entity	P	C	Name
0	X	1	Alice
1	0	2	Robert
2	1	X	Charlie

So the relationship between Robert and Charlie is still exactly as we defined in the scene, without affecting Robert's relationship with Alice.

Interviewer: Fascinating.

MrGVSV

Seems like a simple fix for this. Thanks for the awesome "Visual Journey" explanation— it really helped to make this problem more digestible!

Just a few comments.

MrGVSV · 2023-02-19T20:56:12Z

crates/bevy_ecs/src/reflect.rs

+        (self.map_entities)(world, entity_map, entity_map.values().collect())
+    }
+
+    pub fn map_specific_entities(


Nit: Doc comments would be helpful for helping users understand why they would want to use this method over map_entities.

MrGVSV · 2023-02-19T21:00:02Z

crates/bevy_scene/src/dynamic_scene.rs

+        // Collection of entities that have references to entities in the scene,
+        // that need to be updated to entities in the world.
+        // Keyed by Component's TypeId.
+        let mut entity_mapped_entities = HashMap::default();


Minor Nit: Not super necessary, but I think explicitly specifying the type here can be helpful for review/readability. Otherwise, we either need to load up an editor or mentally track the key/value types.

Oooh definitely!

MrGVSV · 2023-02-19T21:01:20Z

crates/bevy_scene/src/dynamic_scene.rs

+        // Collection of entities that have references to entities in the scene,
+        // that need to be updated to entities in the world.
+        // Keyed by Component's TypeId.
+        let mut entity_mapped_entities = HashMap::default();


Bikeshed: Name could be clearer imo. Maybe scene_mappings?

Yeaah, that makes sense. I updated the comment too to be a little more clear, hopefully.

MrGVSV · 2023-02-19T21:06:39Z

crates/bevy_scene/src/dynamic_scene.rs

+    fn components_not_defined_in_scene_should_not_be_effected_by_scene_entity_map() {
+        // Testing that scene reloading applies EntitiyMap correctly to MapEntities components.
+
+        // First, we create a simple world with a parent and a child relationship


Glad to see such a thorough test complete with helpful step-by-step comments that walk us through the intention behind the test. Thanks for this!

MrGVSV · 2023-02-19T21:07:02Z

crates/bevy_scene/src/dynamic_scene.rs

+    use crate::dynamic_scene_builder::DynamicSceneBuilder;
+
+    #[test]
+    fn components_not_defined_in_scene_should_not_be_effected_by_scene_entity_map() {


Typo:

Suggested change

fn components_not_defined_in_scene_should_not_be_effected_by_scene_entity_map() {

fn components_not_defined_in_scene_should_not_be_affected_by_scene_entity_map() {

Ooooh good catch!

MrGVSV · 2023-02-19T21:18:59Z

crates/bevy_scene/src/dynamic_scene.rs

+            .resource_mut::<AppTypeRegistry>()
+            .write()
+            .register::<Parent>();
+        let gen_0_entity = world.spawn_empty().id();


Nit: This test might be a bit easier to read if these names were a little more descriptive. Maybe like world_entity_a/scene_entity_a or something similar?

Okay, I did orginal_/from_scene_ prefixes, and then explicitly called them parent_entity/child_entity.

Yeah I think this is much clearer. Thanks!

Testare · 2023-02-20T17:11:37Z

Awww, yesterday was my birthday! PR feedback is a great birthday present <3

Applied the feedback, things should be clearer now.

MrGVSV

Just a few comments on documentation stuff.

Happy belated birthday btw! 🥳

MrGVSV · 2023-02-20T18:06:22Z

crates/bevy_ecs/src/reflect.rs

+        (self.map_entities)(world, entity_map, entity_map.values().collect())
+    }
+
+    /// This is like `map_entities`, but only applied to specific entities, not all values in the `EntityMap`.


Nit: You can probably make map_entities and EntityMap links.

MrGVSV · 2023-02-20T18:07:29Z

crates/bevy_ecs/src/reflect.rs

+
+    /// This is like `map_entities`, but only applied to specific entities, not all values in the `EntityMap`.
+    ///
+    /// This is useful for when you only want to apply the `MapEntity` logic to specific entities. For example,


Nit:

Suggested change

/// This is useful for when you only want to apply the `MapEntity` logic to specific entities. For example,

/// This is useful for when you only want to apply the [`MapEntities`] logic to specific entities. For example,

MrGVSV · 2023-02-20T18:17:40Z

crates/bevy_ecs/src/reflect.rs

+    /// a scene can be loaded with `Parent` components, but then a `Parent` component can be added to these entities
+    /// after they have been loaded. If you reload the scene and used plain `map_entities`, those `Parent` components
+    /// with already valid entity references could be updated to point at something else entirely.


This might be a little too specific of an example haha. Maybe we just say something along the lines of how it scopes the mapping to a specific pool of entities so as to preserve the proper relations? Or at least something a bit more general?

Hm, that actually makes me wonder. Do we ever want the standard map_entities function? Would it be better to just merge this function into that one? Then just require users who do want it to do map_entities(&mut world, &entity_map, entity_map.values().collect())?

I may be missing a glaringly obvious reason for keeping it haha but rn it sounds like leaving the door open to footguns.

At first I left this here as a habit from work where if you have a public API that might be consumed by others, leave it alone. But I realize that's not TheBevyWay(tm) right now, so we can go ahead and replace it.

So I used ripgrep to find invocations, and I found out the only other place is in Scene::write_to_world_with (Not DynamicScene::write_to_world_with). This bug does not exist in this case because EntityMap is not passed to this code; it seems you can't do a "reload" with a Scene. I'll modify the code there to pass in the entity values though to use this interface.

MrGVSV · 2023-02-20T18:20:47Z

crates/bevy_ecs/src/reflect.rs

+    /// This is like `map_entities`, but only applied to specific entities, not all values in the `EntityMap`.
+    ///
+    /// This is useful for when you only want to apply the `MapEntity` logic to specific entities. For example,
+    /// a scene can be loaded with `Parent` components, but then a `Parent` component can be added to these entities


Nit: I wonder if we should avoid mentioning "scenes" in bevy_ecs content like this since it's not actually a dependency of the crate. Perhaps we use the concept of "worlds" since scenes are basically a tiny and limited World?

@alice-i-cecile probably has more knowledge in this area

I think that this is probably net useful: humans can figure this out.

Testare · 2023-02-21T17:19:47Z

Sorry for the delay, yesterday was also abnormally busy.

As I said in reply to one of your comments, I think it makes sense just to combine the two methods and remove footguns. There was one other place it was used, in the Scene class, and so I made the adjustment there.

Now that the separate method has been removed, I guess that kinda resolves all the comments about those doc comments. Unless you think I should add a doc comment for the map_entities function itself?

Shatur

I like this change. But I would suggest to keep the original map_entities and rename it into map_all_entities and call your new method map_entities. It will allow to avoid extra Vec allocation.
Also I would use slice instead of &Vec.

MrGVSV · 2023-02-22T01:50:34Z

I like this change. But I would suggest to keep the original map_entities and rename it into map_all_entities and call your new method map_entities. It will allow to avoid extra Vec allocation. Also I would use slice instead of &Vec.

Could we just take in an impl Iterator<Item = Entity> instead of &Vec<Entity>/&[Entity]? That would mean we can pass entity_map.values() directly without collecting into a Vec. And it removes the need for keeping around a possibly-footgunny method.

Testare · 2023-02-22T03:00:19Z

Sadly, I don't think we can do that, because the way ReflectMapEntities is implemented is with an internal closure, and you can't make closures generic over traits. I think I can do it with slices though.

I liked what Shatur suggested though about making a map_entities and a map_all_entities function though. We can use an Option<&[Entity]> in the internal closure, and if the Option is None, we just iterate over the entity_map values in the closure itself. That way we don't need to allocate any Vec's that way either.

Thoughts?

Shatur · 2023-02-22T08:00:20Z

crates/bevy_ecs/src/reflect.rs

-                    if let Some(mut component) = world.get_mut::<C>(entity) {
-                        component.map_entities(entity_map)?;
+            map_entities: |world, entity_map, entities_opt| {
+                if let Some(entities) = entities_opt {


Oh, we can't use unwrap_or_else on entities_opt, right?
Then I would provide two separate methods. It's more explicit and we duplicate loops anyway.

Yeah my rustfu isn't strong enough to know how to unwrap an Option of dyn Iterator<Item=Entity>. I tried to make it prettier but i couldn't without collecting.

We do provide two public methods, but they both call one internal closure. Are you saying you'd prefer we store two closures?

We do provide two public methods, but they both call one internal closure. Are you saying you'd prefer we store two closures?

Yep!

Oh, or do you mean have a separate method for the inner loop that the closure calls?

EDIT: sorry, didn't see you already responded XP

Ok I'll get on that

nicopap

Just a doc request. But otherwise, I'd like to see this merged, it expands the range of things you can use DynamicScene for by quite a lot.

nicopap · 2023-02-22T09:25:35Z

crates/bevy_scene/src/dynamic_scene.rs

-        for registration in type_registry.iter() {
+        // Updates references to entities in the scene to entities in the world
+        for (type_id, entities) in scene_mappings.into_iter() {
+            let registration = type_registry.get(type_id).unwrap();


Looks like unwrapping here is fine, since by construction scene_mappings is populated by registrations that come from the type_registry. But it would be a good idea to make the expectation clear (through a comment), so that in the future, say if previous code changes, and the code panic, the poor soul dealing with the code has a chance of understanding why it happens.

Or better with expect.

Shatur · 2023-02-22T16:10:46Z

crates/bevy_scene/src/dynamic_scene.rs

@@ -115,7 +115,8 @@ impl DynamicScene {

        // Updates references to entities in the scene to entities in the world
        for (type_id, entities) in scene_mappings.into_iter() {
-            let registration = type_registry.get(type_id).unwrap();
+            let registration = type_registry.get(type_id)
+                .expect("This TypeRegistration was where we got the TypeId initially, should be safe to unwrap");


One last minor nitpick. In Rust panic messages usually written differently.

crates/bevy_scene/src/dynamic_scene.rs

nicopap

LGTM

nicopap · 2023-03-07T09:09:18Z

## Migration Guide

- in `bevy_ecs`, `ReflectMapEntities::map_entites` now
  requires an additional `entities` parameter to specify
  which entities it applies to. To keep the old behavior,
  use the new `ReflectMapEntities::map_all_entites` instead.

github-actions · 2023-03-07T09:09:44Z

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

Testare · 2023-03-07T16:21:43Z

Created a PR #7951 for this bugfix without the breaking changes

# Objective Fix a bug with scene reload. (This is a copy of #7570 but without the breaking API change, in order to allow the bugfix to be introduced in 0.10.1) When a scene was reloaded, it was corrupting components that weren't native to the scene itself. In particular, when a DynamicScene was created on Entity (A), all components in the scene without parents are automatically added as children of Entity (A). But if that scene was reloaded and the same ID of Entity (A) was a scene ID as well*, that parent component was corrupted, causing the hierarchy to become malformed and bevy to panic. *For example, if Entity (A)'s ID was 3, and the scene contained an entity with ID 3 This issue could affect any components that: * Implemented `MapEntities`, basically components that contained references to other entities * Were added to entities from a scene file but weren't defined in the scene file - Fixes #7529 ## Solution The solution was to keep track of entities+components that had `MapEntities` functionality during scene load, and only apply the entity update behavior to them. They were tracked with a HashMap from the component's TypeID to a vector of entity ID's. Then the `ReflectMapEntities` struct was updated to hold a function that took a list of entities to be applied to, instead of naively applying itself to all values in the EntityMap. (See this PR comment #7570 (comment) for a story-based explanation of this bug and solution) ## Changelog ### Fixed - Components that implement `MapEntities` added to scene entities after load are not corrupted during scene reload.

Fix a bug with scene reload. (This is a copy of #7570 but without the breaking API change, in order to allow the bugfix to be introduced in 0.10.1) When a scene was reloaded, it was corrupting components that weren't native to the scene itself. In particular, when a DynamicScene was created on Entity (A), all components in the scene without parents are automatically added as children of Entity (A). But if that scene was reloaded and the same ID of Entity (A) was a scene ID as well*, that parent component was corrupted, causing the hierarchy to become malformed and bevy to panic. *For example, if Entity (A)'s ID was 3, and the scene contained an entity with ID 3 This issue could affect any components that: * Implemented `MapEntities`, basically components that contained references to other entities * Were added to entities from a scene file but weren't defined in the scene file - Fixes #7529 The solution was to keep track of entities+components that had `MapEntities` functionality during scene load, and only apply the entity update behavior to them. They were tracked with a HashMap from the component's TypeID to a vector of entity ID's. Then the `ReflectMapEntities` struct was updated to hold a function that took a list of entities to be applied to, instead of naively applying itself to all values in the EntityMap. (See this PR comment #7570 (comment) for a story-based explanation of this bug and solution) - Components that implement `MapEntities` added to scene entities after load are not corrupted during scene reload.

nicopap · 2023-04-22T08:42:25Z

Now that #7951 is merged, you need to resolve the conflicts, it should be ready for final review/merge.

alice-i-cecile · 2023-05-01T19:16:02Z

@Testare once you've updated your PR description I'm happy to merge this :) The rename still looks nice.

Testare · 2023-05-01T19:40:24Z

@alice-i-cecile Updated PR description =]

alice-i-cecile · 2023-05-01T21:40:15Z

Excellent PR description: really clearly motivates the change and provides a helpful migration guide. Thanks!

Testare · 2023-05-01T21:48:05Z

Awww, thanks =]

Fix a bug with scene reload. (This is a copy of bevyengine#7570 but without the breaking API change, in order to allow the bugfix to be introduced in 0.10.1) When a scene was reloaded, it was corrupting components that weren't native to the scene itself. In particular, when a DynamicScene was created on Entity (A), all components in the scene without parents are automatically added as children of Entity (A). But if that scene was reloaded and the same ID of Entity (A) was a scene ID as well*, that parent component was corrupted, causing the hierarchy to become malformed and bevy to panic. *For example, if Entity (A)'s ID was 3, and the scene contained an entity with ID 3 This issue could affect any components that: * Implemented `MapEntities`, basically components that contained references to other entities * Were added to entities from a scene file but weren't defined in the scene file - Fixes bevyengine#7529 The solution was to keep track of entities+components that had `MapEntities` functionality during scene load, and only apply the entity update behavior to them. They were tracked with a HashMap from the component's TypeID to a vector of entity ID's. Then the `ReflectMapEntities` struct was updated to hold a function that took a list of entities to be applied to, instead of naively applying itself to all values in the EntityMap. (See this PR comment bevyengine#7570 (comment) for a story-based explanation of this bug and solution) - Components that implement `MapEntities` added to scene entities after load are not corrupted during scene reload.

alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Scenes Serialized ECS data stored on the disk A-ECS Entities, components, systems, and events labels Feb 8, 2023

bors bot added a commit that referenced this pull request Feb 8, 2023

Try #7570:

086c731

Testare mentioned this pull request Feb 9, 2023

Make scene handling of entity references robust #7335

Merged

MrGVSV self-requested a review February 16, 2023 06:46

jakobhellermann self-requested a review February 16, 2023 21:38

MrGVSV reviewed Feb 19, 2023

View reviewed changes

MrGVSV reviewed Feb 20, 2023

View reviewed changes

Shatur added this to the 0.10 milestone Feb 22, 2023

Shatur suggested changes Feb 22, 2023

View reviewed changes

Shatur reviewed Feb 22, 2023

View reviewed changes

nicopap suggested changes Feb 22, 2023

View reviewed changes

Shatur reviewed Feb 22, 2023

View reviewed changes

crates/bevy_scene/src/dynamic_scene.rs Outdated Show resolved Hide resolved

Shatur approved these changes Feb 22, 2023

View reviewed changes

nicopap approved these changes Feb 22, 2023

View reviewed changes

nicopap modified the milestones: 0.10.1, 0.11 Mar 7, 2023

nicopap added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Mar 7, 2023

Testare mentioned this pull request Mar 7, 2023

Bugfix: Scene reload fix (nonbreaking) #7951

Merged

Rename map_entities and map_specific_entities

686c80f

Testare force-pushed the scene_reload_fix branch from b242a02 to 686c80f Compare May 1, 2023 19:00

alice-i-cecile removed X-Controversial There is active debate or serious implications around merging this PR S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels May 1, 2023

alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change and removed C-Bug An unexpected or incorrect behavior M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels May 1, 2023

Testare changed the title ~~Bugfix: Scene reloading corruption~~ Rename map_entities and map_specific_entities May 1, 2023

alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 1, 2023

alice-i-cecile approved these changes May 1, 2023

View reviewed changes

alice-i-cecile added this pull request to the merge queue May 1, 2023

Merged via the queue into bevyengine:main with commit a29d328 May 1, 2023

This was referenced Jun 28, 2023

News: Release 0.11 (wrong branch ... see #657) bevyengine/bevy-website#656

Closed

News: Release 0.11 bevyengine/bevy-website#657

Merged

	fn components_not_defined_in_scene_should_not_be_effected_by_scene_entity_map() {
	fn components_not_defined_in_scene_should_not_be_affected_by_scene_entity_map() {

	/// This is useful for when you only want to apply the `MapEntity` logic to specific entities. For example,
	/// This is useful for when you only want to apply the [`MapEntities`] logic to specific entities. For example,

Rename map_entities and map_specific_entities #7570

Rename map_entities and map_specific_entities #7570

Conversation

Testare commented Feb 8, 2023 • edited Loading

Objective

Solution

Changelog

Changed

Migration Guide

Testare commented Feb 8, 2023

JMS55 commented Feb 8, 2023 • edited Loading

bors bot commented Feb 8, 2023

try

Illiux commented Feb 11, 2023 • edited Loading

Testare commented Feb 11, 2023

Illiux commented Feb 11, 2023

Illiux commented Feb 11, 2023

Testare commented Feb 12, 2023

Testare commented Feb 16, 2023 • edited Loading

The Scene-Reload Bug In 3 Acts: A Visual Journey

Act I: Setting Up

Act II: The Bug Revealed

Act III: Happily ever After

Interview with the Author: Handling Multiple Component Types

MrGVSV left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Testare commented Feb 20, 2023

MrGVSV left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Testare commented Feb 21, 2023

Shatur left a comment

Choose a reason for hiding this comment

MrGVSV commented Feb 22, 2023

Testare commented Feb 22, 2023 • edited Loading

Shatur Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Testare Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

nicopap left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shatur Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

nicopap left a comment

Choose a reason for hiding this comment

nicopap commented Mar 7, 2023

github-actions bot commented Mar 7, 2023

Testare commented Mar 7, 2023

nicopap commented Apr 22, 2023 • edited Loading

alice-i-cecile commented May 1, 2023

Testare commented May 1, 2023 • edited Loading

alice-i-cecile commented May 1, 2023

Testare commented May 1, 2023

Testare commented Feb 8, 2023 •

edited

Loading

JMS55 commented Feb 8, 2023 •

edited

Loading

Illiux commented Feb 11, 2023 •

edited

Loading

Testare commented Feb 16, 2023 •

edited

Loading

Testare commented Feb 22, 2023 •

edited

Loading

Shatur Feb 22, 2023 •

edited

Loading

Testare Feb 22, 2023 •

edited

Loading

nicopap left a comment •

edited

Loading

Shatur Feb 22, 2023 •

edited

Loading

nicopap commented Apr 22, 2023 •

edited

Loading

Testare commented May 1, 2023 •

edited

Loading